Skip to content

C++/C#/Java: localFlowStep performance fix#1808

Closed
jbj wants to merge 1 commit into
github:masterfrom
jbj:localFlowStep0
Closed

C++/C#/Java: localFlowStep performance fix#1808
jbj wants to merge 1 commit into
github:masterfrom
jbj:localFlowStep0

Conversation

@jbj

@jbj jbj commented Aug 23, 2019

Copy link
Copy Markdown
Contributor

I have a configuration that's very slow without this fix:

OverflowBuffer.ql-17:DataFlowImplLocal::localFlowStep#fff ... 16.3s

After this fix (the localFlowStep0 predicate was called localFlowStepCandidate when I measured this):

OverflowBuffer.ql-17:DataFlowImplLocal::localFlowStepCandidate#fff ... 1.1s
OverflowBuffer.ql-17:DataFlowImplLocal::localFlowStep#fff ............ 767ms

The slow RA looks like this:

3371506 ~0%     {3} r1 = SCAN DataFlowUtil::simpleLocalFlowStep#ff@staged_ext OUTPUT FIELDS {"FieldConfiguration",DataFlowUtil::simpleLocalFlowStep#ff@staged_ext.<0>,DataFlowUtil::simpleLocalFlowStep#ff@staged_ext.<1>}
3371506 ~0%     {3} r2 = r1 AND NOT DataFlowImplLocal::outBarrier#ff#cpe#1_0_"FieldConfiguration"#antijoin_rhs(r1.<1>,"FieldConfiguration")
3011261 ~0%     {3} r3 = r2 AND NOT DataFlowImplLocal::fullBarrier#ff(r2.<1>,"FieldConfiguration")
2987019 ~0%     {3} r4 = r3 AND NOT DataFlowImplLocal::fullBarrier#ff(r3.<2>,"FieldConfiguration")
2987019 ~0%     {3} r5 = SCAN r4 OUTPUT FIELDS {r4.<1>,r4.<2>,"FieldConfiguration"}
               return r5

The fast RA looks like this:

3371506 ~0%     {3} r1 = SCAN DataFlowUtil::simpleLocalFlowStep#ff@staged_ext OUTPUT FIELDS {DataFlowUtil::simpleLocalFlowStep#ff@staged_ext.<0>,DataFlowUtil::simpleLocalFlowStep#ff@staged_ext.<1>,"FieldConfiguration"}
               return r1
[2019-08-23 10:28:55] (1s) Registering DataFlowImplLocal::localFlowStepCandidate#fff + [] with content 22b8729e74bcvurf9ne01sv01vnf5
[2019-08-23 10:28:55] (1s)  >>> Relation DataFlowImplLocal::localFlowStepCandidate#fff: 3371506 rows using 0 MB
...
3371506 ~0%     {3} r1 = DataFlowImplLocal::localFlowStepCandidate#fff AND NOT DataFlowImplLocal::outBarrier#ff#cpe#1_0_"FieldConfiguration"#antijoin_rhs(DataFlowImplLocal::localFlowStepCandidate#fff.<0>,DataFlowImplLocal::localFlowStepCandidate#fff.<2>)
3011261 ~0%     {3} r2 = r1 AND NOT DataFlowImplLocal::fullBarrier#ff(r1.<0>,r1.<2>)
2987019 ~0%     {3} r3 = r2 AND NOT DataFlowImplLocal::fullBarrier#ff(r2.<1>,r2.<2>)
               return r3
[2019-08-23 10:28:56] (2s) Registering DataFlowImplLocal::localFlowStep#fff + [] with content 229f2ea1d4qq9c239j039cjlm413v
[2019-08-23 10:28:56] (2s)  >>> Relation DataFlowImplLocal::localFlowStep#fff: 2987019 rows using 0 MB

To reproduce this problem, apply #1787 and run OverflowBuffer.ql on the MySQL snapshot at https://jenkins.internal.semmle.com/job/Language-Tests/job/CPP-HeaderQueryTests/.

@jbj jbj requested review from a team as code owners August 23, 2019 12:34
@aschackmull

aschackmull commented Aug 29, 2019

Copy link
Copy Markdown
Contributor

I think @hmakholm was looking at a simple optimiser fix that could make this change superfluous, so I'd rather not merge this just now.

@hmakholm

hmakholm commented Aug 29, 2019

Copy link
Copy Markdown
Contributor

I think @hmakholm was looking at a simple optimiser fix

Indeed -- it's been almost ready to go for some time, except waiting on benchmarking resources released freed from a project with priority.

@jbj

jbj commented Aug 30, 2019

Copy link
Copy Markdown
Contributor Author

I can confirm that @hmakholm's PR fixes the evaluator performance, so there's no need for this workaround.

@jbj jbj closed this Aug 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@aschackmull aschackmull

Labels

C# C++ Java

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jbj @aschackmull @hmakholm